-
Notifications
You must be signed in to change notification settings - Fork 16.7k
chore: 100% test coverage for SQL parsing #33568
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment
|
9307e30 to
e151a54
Compare
e151a54 to
cf5c8ac
Compare
094e4b9 to
eefc631
Compare
9d654ac to
b1830b2
Compare
| target = statements[-1] | ||
| for node in statements[-1].walk(): | ||
| if hasattr(node, "comments"): | ||
| if hasattr(node, "comments"): # pragma: no cover |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were a couple lines that I couldn't hit with tests, but I thought it was safer to keep them, so I added the pragma.
| rendered_template = Template(template).render(processor.get_context()) | ||
| code = processor.env.compile(ast) | ||
| template = Template.from_code(processor.env, code, globals=processor.env.globals) | ||
| rendered_sql = template.render(processor.get_context()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made sure to include the fix from #33596.
eefc631 to
dcd9140
Compare
4834814 to
1c2fee5
Compare
789c101 to
7be3f1b
Compare
1c2fee5 to
a015ab1
Compare
next on my wish list -> |
7be3f1b to
2032f8c
Compare

SUMMARY
Part of SIP-117, stacked on:
Adds 100% unit tests coverage to the
superset/sql/directory, and a CI rule to enforce it.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION